-
Notifications
You must be signed in to change notification settings - Fork 245
feat(pulse): optimize gas while keeping requests on-chain #2519
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
4 Skipped Deployments
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. Can we expand the benchmark a bit? Include 2-3 different scenarios like:
1- Making a request for 1-2-4-8 different feeds
2- Filling each of these requests
} | ||
|
||
struct ProviderInfo { | ||
uint128 baseFeeInWei; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why this particular ordering if we can pack it even more?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont think we can do better than 3 slots for ProviderInfo unless i'm missing something?
@m30m @ali-bahjati round 2! Expanded the gas benchmarks:
Updated the Pyth gas benchmarks as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Summary
Gas optimizations to the Pulse contract that keep the requests on-chain.
Rationale
The motivation behind this PR is to see how cheap we can get the basic request/fulfill flow while still keeping full request state on-chain. This will let us treat the contract as the source of truth for state, greatly simplifying keeper implementation. It also eliminates a class of errors related to tracking chain state, improving reliability.
This is an alternative design to the gas optimizations done in #2492 , which make the opposite tradeoff: move request state off-chain (specifically the priceIds array) to gain gas savings at the cost of keeper complexity.
Results:
https://docs.google.com/spreadsheets/d/1AYcmnwDQp2OA8n0xfoTf-GtThmhsX1ahxMQilIua4b0/edit?gid=0#gid=0
Optimizations made:
bytes32[MAX_PRICE_IDS] to bytes8[]
. There is very low chance of collision at 8 bytes, and even at 4 bytes, so we can go even lower here.P ~= (n(n-1))/(2 * 2^64)
(where P is the chance of a collision for n entries) gives 2.7e-12, which is very unlikely.callbackGasLimit
from uint256 to uint32, since this number can't exceed the block gas limit anywayexclusivityPeriod
from uint256 to uint32, since we only support requests 60s into the future, and 32 bytes still lets us represent ~49k days in seconds.pythFeeInWei
,baseFeeInWei
,feePerFeedInWei
,feePerGasInWei
from uint128 to uint96 since 96 bytes still gives us the ability to represent up to ~79,228,162,514 ETH in wei. We could go lower here.Notes:
firstUnfulfilledSeq
feature, as called out in the code: